-
Notifications
You must be signed in to change notification settings - Fork 605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ONVIF client #1589
base: master
Are you sure you want to change the base?
fix: ONVIF client #1589
Conversation
I have the same issue. @AlexxIT could you merge it? |
PR will be accepted after a careful review. |
Can you fix or split PR? |
@AlexxIT you didn't ask but I would like to advocate in @seydx's favor: I think one of the best things about go2rtc is how fault-tolerant and universal it is. In the ideal world, all camera manufacturers would implement things following best practices or would release firmware updates fixing stuff. But real world is very different, in real world you buy a camera and almost never receive a firmware update for it. And it's amazing how go2rtc works great for the real world. I don't even have any Tapo cameras, and maybe Tapo is one of the good manufacturers which fixes issues reported by their customers. But it seems to me that @seydx's fix is generic enough to handle potential issues like this for any future camera that may send malformed responses. From what I can tell, it doesn't seem that the fix will have a performance penalty. Also, the code remains simple to me. Maybe the request logic could be encapsulated in a new class to keep ONVIF client class tidy. And then maybe unit tests could be added to such class, especially to guarantee that it continues to work for previous cameras. But I'm not sure. Anyway, I'm also in favor of splitting this PR into two. |
There are many fixes for broken protocols in go2rtc. Just specifically in this case there is just too much code for a fairly useless situation - ONVIF from some fw from some Tapo cameras. I've had to use a raw HTTP client elsewhere on go2rtc before. Perhaps after refactoring and merging this code - the fix can be returned. It is even better to write a test on this fix, so that the crooked response from the camera is fixed there. |
I updated my setup from v1.9.7 to v1.9.8 the other day and faced the same issue - or at least I think it is the same. The changes in commit f601c47, "Improve ONVIF server", removed the optional "Category" field from the "GetCapabilities" requests, which makes my Tapo CS320WS send garbage. Adding back the "Category" field fixes it. I don't know if it is a better fix, but it is at least less code.
|
@ostehamster fixed 57cd791 |
This PR fixes two ONVIF camera compatibility issues:
Testing: Verified working with cameras that previously failed with: